Skip to content

[trello.com/c/4FomJzhO] Add Copy action to Failed messages menu + Open menu on long tap #833

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 7 commits into
base: develop
Choose a base branch
from

Conversation

Lainaaa
Copy link
Collaborator

@Lainaaa Lainaaa commented Apr 22, 2025

​While working on the task, I noticed some shortcomings that initially seemed minor. However, due to the cell system, they turned out to be significant, leading to a considerable amount of boilerplate code. I hope I managed to reduce this boilerplate by extracting the copy functionality and the animated press effect, introduced by Vladimir, into separate methods. Additionally, I aimed to improve the code by relocating functions unrelated to ChatMenuManager into the cell.

@Lainaaa Lainaaa self-assigned this Apr 22, 2025
@art-divin
Copy link
Collaborator

/run-tests

Lainaaa added 5 commits April 23, 2025 14:18
# Conflicts:
#	Adamant/Modules/Chat/View/Subviews/ChatBaseMessage/ChatMessageCell.swift
#	Adamant/Modules/Chat/View/Subviews/ChatMedia/ChatMediaCell.swift
#	Adamant/Modules/Chat/View/Subviews/ChatReply/ChatMessageReplyCell.swift
#	Adamant/Modules/Chat/View/Subviews/ChatTransaction/Container/ChatTransactionContainerView.swift
@Lainaaa Lainaaa marked this pull request as ready for review April 24, 2025 12:32
@adamantmm
Copy link
Member

Don’t merge to dev it because of many changes.

It’s for release v3.12.

@art-divin art-divin requested a review from Copilot May 28, 2025 20:01
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR refactors the copy functionality and long press gesture handling across multiple chat components to reduce boilerplate code while adding support for failed message interactions.

  • Extracts copy-to-pasteboard behavior into shared dialog methods.
  • Consolidates long press gesture handling via a common GestureHelper interface.
  • Updates property names and visibility modifiers to better reflect intended usage.

Reviewed Changes

Copilot reviewed 20 out of 20 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
CommonKit/Helpers/TaskManager.swift Made TaskManager API public and updated header comments.
CommonKit/Helpers/GestureHelper.swift Introduced a protocol extension for processing long press gestures.
AdamantDialogService.swift & DialogService.swift Added and integrated a new copyToPasteboard method.
PartnerQRViewModel.swift, ChatViewModel.swift Updated copy actions to use the new dialogService methods.
Various Chat module files Refactored long press and copy gesture handling to use copyAction closures and GestureHelper.
ChatMenuManager.swift, ChatDialogManager.swift, ChatDataSourceManager.swift, ChatAction.swift, ChatFactory.swift Updated state management and action handling for failed message scenarios and consistent copy actions.
Comments suppressed due to low confidence (1)

Adamant/Modules/Chat/View/Subviews/ChatTransaction/Container/ChatTransactionContainerView.swift:133

  • [nitpick] Property names in Swift should follow lowerCamelCase. Consider renaming 'GestureTaskManager' to 'gestureTaskManager' for consistency with Swift style guidelines.
var GestureTaskManager: TaskManager = TaskManager()

Comment on lines +161 to +162
self?.cellContainerView.animatePressDown() },
onGestureEnded: { [weak self] in self?.messageContainerView.animatePressUp() }
Copy link
Preview

Copilot AI May 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The onGestureEnded closure in handleLongPressToOpenMenu references 'messageContainerView', but ChatMediaCell defines 'cellContainerView'. Please update it to use the correct property for consistency.

Suggested change
self?.cellContainerView.animatePressDown() },
onGestureEnded: { [weak self] in self?.messageContainerView.animatePressUp() }
self?.cellContainerView.animatePressDown() },
onGestureEnded: { [weak self] in self?.cellContainerView.animatePressUp() }

Copilot uses AI. Check for mistakes.

}

extension ChatMessageCell: ChatMenuManagerDelegate {
var isFailedMessage: Bool {
model.backgroundColor == .failed
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generally speaking ,deciding on the state based on backgroundColor is not a future-proof solution. Would you consider improving this part a bit?

model.backgroundColor == .failed
}

func showFailedMenu(){
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

showFailedMenuAlert ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants